review: bump installed reviewer to review-v1.7.0 - #276
Merged
Conversation
…1.6.1; enable flip-gated re-reviews The installed copy (.github/workflows/review.md) was still at review-v1.4.1, the version gh aw add pinned on install (#241), and had never been updated: five releases behind the shared source. Found via PR #266 review 4739236283, a REQUEST_CHANGES with the bare body "Changes requested — see inline comments." and zero inline comments; v1.5.0's code-rendered re-review accountability section (#244) exists to name the still-open blocking thread in exactly that case, and v1.6.1 carries the #262 fix that keeps a blocking verdict from silently downgrading to COMMENTED when the body is empty. gh aw update cannot perform this bump: gh-aw v0.81.6 resolves a tag ref only when isSemanticVersionTag() accepts it, which rejects this repo's changesets-style prefixed tags (review-v1.4.1), falls through to branch resolution, and 404s. This commit replicates the documented 3-way merge by hand (base review-v1.4.1, ours the installed copy, theirs review-v1.6.1) and recompiles with gh aw compile. All KHAN/ACTIONS LOCAL OVERRIDE edits are preserved: the fork guard, the disabled observability block (the GH_AW_OTEL_SENTRY_* secrets still do not exist in this repo or the org), and max-ai-credits 2500, now mirrored into the new REVIEW_MAX_AI_CREDITS env var per the KEEP IN SYNC rule the upstream frontmatter added. Also sets the v1.6.0 re-review mode dial to flip-gated in .github/aw/review/ROUTING: repeat reviews run thread reconciliation plus the correctness pass over new hunks, with a REQUEST_CHANGES -> APPROVE flip vetoed by any validated blocking finding; the first full review of a ready PR and the 0.4 divergence tripwire keep full-depth coverage. Verified against the real parser: reReviewMode flip-gated, zero warnings.
|
khan-actions-bot
requested review from
a team,
jeresig and
kevinb-khan
and removed request for
a team
July 20, 2026 23:55
Contributor
Author
Follow-up:
|
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…t the re-review mode dial out Review feedback on the v1.6.1 bump: - review-pins.test.ts now diffs the installed .github/workflows/review.md against the shared source at the pinned release tag (fetching the tag when the checkout is shallow) and fails if any hunk lacks a KHAN/ACTIONS LOCAL OVERRIDE marker. gh aw update cannot resolve changesets-style tags, so bumps here are manual 3-way merges; the pin tests checked version consistency but nothing verified the merged content, and the next manual bump could silently drop an override or an upstream hunk. - Drop the flip-gated ROUTING change: the motivating defect (the empty REQUEST_CHANGES body on #266) is fixed by the version bump alone, and the mode dial is a separable cost/depth decision; splitting it keeps the bump revertable and the behavior change independently attributable. It follows in its own PR, starting at scoped per the same review.
This comment has been minimized.
This comment has been minimized.
| ); | ||
| expect( | ||
| unmarked.map((hunk) => hunk.slice(0, 8).join("\n")), | ||
| ).toEqual([]); |
Contributor
There was a problem hiding this comment.
suggestion (non-blocking): The hunk-parse and marker-filter logic is asserted only against the real, currently-clean files (5 marked hunks today), so a future refactor that breaks the @@-split or the marker substring match would still pass green — and the next silently-dropped override would slip through the guard built to catch it. Consider a fixture-driven negative case that feeds synthetic diff -u output containing one marked and one unmarked hunk and asserts only the unmarked one survives, so the detection path itself is exercised.
Low-confidence (1)
.github/workflows/review-pins.test.ts:64— Open question: could release automation publish a tag/aliasgh aw updatecan resolve, retiring the bespoke guard plus the manual 3-way merge on every bump? Not a defect.
| hunks.at(-1)?.push(line); | ||
| } | ||
| } | ||
| const unmarked = hunks.filter( |
Contributor
There was a problem hiding this comment.
thought (non-blocking): The marker requirement is per-hunk, not per-edit: a hunk is exonerated if KHAN/ACTIONS LOCAL OVERRIDE appears anywhere in it, and diff -u folds edits within ~3 context lines into one hunk — so an unmarked edit adjacent to an existing marker (e.g. right after max-ai-credits: 2500) can ride through undetected. The docstring already discloses this, so it is a documented tradeoff. If you ever want the stronger "every divergence is individually justified" guarantee, committing the pinned source as a snapshot/patch and asserting byte-exactness would close it (and would also make the test hermetic — see the fetch note).
| // just the pinned tag, then retry. | ||
| spawnSync( | ||
| "git", | ||
| ["fetch", "--quiet", "--depth=1", "origin", "tag", ref], |
Contributor
There was a problem hiding this comment.
note (non-blocking): This fetch looks like the steady-state CI path, not a rare fallback: node-ci.yml checks out with actions/checkout@v5 and no fetch-depth, so the default depth-1 shallow clone carries no tags and git show <tag>:... fails on essentially every run — making the network git fetch run each time (a flake surface for a unit test). Committing the pinned source as a snapshot instead of fetching the tag would make this hermetic.
| */ | ||
| describe("installed review.md content vs the pinned source", () => { | ||
| const repoRoot = path.resolve( | ||
| new URL(".", import.meta.url).pathname, |
Contributor
There was a problem hiding this comment.
note (non-blocking): new URL(".", import.meta.url).pathname is not a portable filesystem path (URL percent-encoding for e.g. spaces; a leading-slash-drive form on Windows), and it is the only .pathname-off-a-file-URL in the repo — this same file otherwise passes new URL("./review.md", import.meta.url) straight to fs (line 24). It works on Linux CI today. If you want the idiomatic form:
import {fileURLToPath} from "node:url";
const repoRoot = path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
"../..",
);
jeresig
approved these changes
Jul 21, 2026
…ew-v1.7.0 Re-ran the 3-way merge against the fresh release (#278: the Fable-5 correctness reviewer, the recap label fix + collapsed non-blocking recap with fail-closed unknown labels, and the unbounded-read lens). One conflict, the ref: pin, resolved override-comment + new version; source:/ref: both at review-v1.7.0; gh aw compile clean. The content guard verifies every remaining hunk vs the v1.7.0 tag carries a LOCAL OVERRIDE marker; full suite 1016 passing.
jwbron
changed the title
jwbron
added a commit
that referenced
this pull request
Jul 21, 2026
…is repo Split out of the v1.6.1 bump (#276) per its review: the bump alone fixes the motivating defect, so the mode dial lands separately and independently attributable. Scoped rather than flip-gated, also per review: this repo's composite actions run in consuming repos' CI with their credentials, and under flip-gated a small post-review push under the divergence tripwire would get only the correctness pass, with none of the enabled whole-change reviewers seeing the new hunks. Flip-gated remains the graduation step once re-reviews here show scoped is overpaying. Validated with the real parser: reReviewMode scoped, zero warnings.
Contributor
Review Guidancegithub-actions (1 file)
Excluded from review (1 file)Not individually reviewed — generated, formatting-only, or fully explained by a common pattern above:
|
Contributor
There was a problem hiding this comment.
Approved — no blocking issues found.
4 of 4 prior review threads are still unaddressed as of e2857d8:
4 non-blocking threads still open
- thought (non-blocking)
.github/workflows/review-pins.test.ts:132: The marker requirement is per-hunk, not per-edit: a hunk is exonerated if KHAN/ACTIONS LOCAL OVERRIDE appears anywhere i... - suggestion (non-blocking)
.github/workflows/review-pins.test.ts:140: The hunk-parse and marker-filter logic is asserted only against the real, currently-clean files (5 marked hunks today),... - note (non-blocking)
.github/workflows/review-pins.test.ts:77: new URL(".", import.meta.url).pathname is not a portable filesystem path. It works on Linux CI today. Consider fileURLTo... - note (non-blocking)
.github/workflows/review-pins.test.ts:95: This fetch looks like the steady-state CI path, not a rare fallback: node-ci.yml checks out with actions/checkout@v5 and...
jwbron
added a commit
that referenced
this pull request
Jul 21, 2026
> Stacked on #276 (the v1.7.0 bump); land after it, ideally after observing a re-review or two at the default full depth on v1.7.0 so the mode change stays independently attributable. Split out of #276 per its review. ## What Sets `re-review scoped` in `.github/aw/review/ROUTING`: repeat reviews of a PR run thread reconciliation plus the full enabled roster (holistic, completeness, test-adequacy, first-principles, conventions) over the new-hunks staging. The first full review of a ready PR still runs everything, and the 0.4 divergence tripwire re-arms full review when a push rewrites enough of the PR. ## Why scoped, not flip-gated Per the review on #276: under `flip-gated`, a small post-review push gets only thread-reconciler plus the correctness pass; on a repo whose own ROUTING rationale is that composite actions run in consuming repos' CI with their credentials, the enabled whole-change reviewers should keep seeing new hunks. No specialist lens is enabled here, so the roster delta between the modes is exactly those whole-change reviewers. `flip-gated` stays available as the graduation step once re-reviews here show `scoped` is overpaying. ## Verification - ROUTING parsed with the real `parseRoutingConfig`: `reReviewMode: "scoped"`, zero warnings. - No changeset needed: `.github/` only. Author: jwbron Reviewers: github-actions[bot], jeresig, jaredly, somewhatabstract Required Reviewers: Approved By: github-actions[bot], jeresig Checks: ✅ 9 checks were successful Pull Request URL: #277
This was referenced Jul 21, 2026
jwbron
added a commit
that referenced
this pull request
Aug 3, 2026
🖍 _This is an audit!_ 🖍 ## Summary: ## Why The reviewer installed on this repo (`.github/workflows/review.md`) has been pinned at `review-v1.7.0` since #276 (2026-07-21), four releases behind the shared source. Everything the shared package has shipped since then runs in consuming repos but not here, including one change that makes an existing setting in our own `ROUTING` inert: - **v1.8.0 patch 034181f**: gh-aw's safe-output sanitizer strips XML/HTML comments, so the hidden fingerprint stamp a review body carries never reached the PR; every re-review planned `no-prior-fingerprint` and escalated to full depth. Our `ROUTING` has said `re-review scoped` since #277, and on v1.7.0 that dial does nothing. The plan CLI now falls back to the Step 9 cache-memory record and reports `stampSource`. - **v1.8.0**: the deterministic-orchestrator slices land. Staging becomes a `pre-agent-steps:` step (`lib/stage-pr.ts`), scripted dispatch becomes the only mode (the ROUTING `dispatch` dial is retired; we never set it), Steps 4-6 become code (`lib/submission-plan.ts`), and the dispatch-conformance gate blocks a verdict whose sub-agent outputs do not exist (the v1.7.0 acceptance trial caught the orchestrator submitting a REQUEST_CHANGES after dispatching zero sub-agents). - **v1.9.0/v1.10.0**: open-thread suppression actually fires (it was unreachable on every conforming run, so re-reviews re-posted findings an open bot thread already tracked), `threads.json` / `human-threads.json` are staged by code, and a suppression is attributed to its best-matching thread rather than the first one it clears. - **v1.11.0**: a sub-agent the provider blocks is named as a refusal rather than "malformed output", failure detail and per-agent tool-call counts are kept, and a refused reviewer falls back to `claude-opus-4-8` instead of silently costing coverage. Also picked up: Gerald `.github/NOTIFIED` support, the per-lens consumer payload seam, and the `documentation` reviewer (opt-in; not enabled here). ## Why not `gh aw update` Same as #276: gh-aw's `resolveLatestRef` rejects changesets-style prefixed tags (`review-v1.11.0`) as non-semver, falls through to branch resolution, and 404s. Updates of this workflow stay manual. ## What this PR does - Replicates `gh aw update`'s 3-way merge by hand: base = `review-v1.7.0` source, ours = installed copy, theirs = `review-v1.11.0` source (identical to current main), then `gh aw compile review`. - **Two local overrides retire, because upstream now carries them.** `timeout-minutes: 40` is the shared default as of v1.8.0 (82af000), and the `sandbox.agent.version: v0.27.27` pin plus the `models:` claude-fable-5 pricing block were removed at source in 98f686f. Both override comments said they went away at this bump; they do. - **The remaining differences are the documented overrides and nothing else**, enforced by `review-pins.test.ts`: the same-repo fork guard in `if:` and its `roles: all` comment (public-repo hardening), the commented-out `observability:` block (the `GH_AW_OTEL_SENTRY_*` secrets still exist neither on this repo nor at org level), `max-ai-credits: 2500` with its `REVIEW_MAX_AI_CREDITS` mirror, and the comment on the lib checkout `ref:`. - `source:` and the lib checkout `ref:` both move to `review-v1.11.0` in lockstep, and the recompiled lock picks up the new pre-agent staging step, the dispatcher's `npm ci`, the dispatch-conformance gate `post-steps` step, and `BASH_MAX_TIMEOUT_MS: 1200000` (the blocking dispatcher call). - No consumer-config change is needed: `.github/aw/review/ROUTING` carries no retired `dispatch` line, no `correctness-checks.md` alias to migrate, and the new `documentation` reviewer stays off until a repo adds `enable documentation`. ## Verification - `pnpm test`: 1618 tests across 69 files pass, including `review-pins.test.ts` (source/ref/lock literals all `review-v1.11.0`, and every hunk against the pinned source carries a `KHAN/ACTIONS LOCAL OVERRIDE` marker) and `version-sync.test.ts`. - `pnpm typecheck`: clean. - `gh aw compile review`: 0 errors, 0 warnings. - No changeset needed: both files are under `.github/`, the check's default exclusion, and the shared `workflows/review` package is untouched. Author: jwbron Auditors: jeresig, kevinb-khan Required Reviewers: Approved By: Checks: ⏭️ 8 checks have been skipped, ⌛ 1 check is pending, ✅ 2 checks were successful Pull Request URL: #318
jwbron
added a commit
that referenced
this pull request
Aug 5, 2026
🖍 _This is an audit!_ 🖍 ## Summary: ## Why The reviewer installed on this repo (`.github/workflows/review.md`) was pinned at `review-v1.11.0`; #326 released review@1.12.0, so the shared package's cross-source dedup tier 2 runs in consuming repos but not here. What v1.12.0 adds, and what this bump picks up: - The `claim-clusterer` sub-agent (sonnet, read-only, identity judgment only) names the candidate comments that describe ONE defect, and `dedup.ts` verifies that assertion and merges them, so one problem found by several sources posts once. Tier 2 requires no line agreement, so the same-defect-different-anchor shape is mergeable for the first time; every merge rule stays in code, only a NON-BLOCKING copy may be absorbed on the model's word, and the survivor is always the highest-severity copy. - Dispatch is conditional (only when the candidates hold a legally mergeable pair), and a missing definition or unusable reply leaves the run on tier 1, i.e. today's behavior. - The prompt's Step 3 dedup description gains the clusterer and the `clustering` block in the dispatch result. ## Why not `gh aw update` Same as #318 and #276: gh-aw's `resolveLatestRef` rejects changesets-style prefixed tags (`review-v1.12.0`) as non-semver, falls through to branch resolution, and 404s. Updates of this workflow stay manual. ## What this PR does - Replicates `gh aw update`'s 3-way merge by hand: base = `review-v1.11.0` source, ours = installed copy, theirs = `review-v1.12.0` source, then `gh aw compile review`. The only conflict was the lib checkout `ref:` line inside its override comment block; resolved as comment plus new ref. - `source:` and the lib checkout `ref:` move to `review-v1.12.0` in lockstep. The recompiled lock changes only those literals plus its metadata hashes: the prompt body reaches the runner through `{{#runtime-import .github/workflows/review.md}}` and inline sub-agents are restored from the activation artifact, so the new agent section does not appear in the lock. - The local overrides are unchanged and still the only differences from the pinned source, enforced by `review-pins.test.ts`: the same-repo fork guard in `if:` and its `roles: all` comment (public-repo hardening), the commented-out `observability:` block (the `GH_AW_OTEL_SENTRY_*` secrets still exist neither on this repo nor at org level), `max-ai-credits: 2500` with its `REVIEW_MAX_AI_CREDITS` mirror, and the comment on the lib checkout `ref:`. - No consumer-config change is needed: `.github/aw/review/ROUTING` needs no dial for clustering, and the roster it enables is untouched. ## Verification - `pnpm vitest run`: 1655 tests across 71 files pass, including `review-pins.test.ts` (source/ref/lock literals all `review-v1.12.0`, and every hunk against the pinned source carries a `KHAN/ACTIONS LOCAL OVERRIDE` marker) and `version-sync.test.ts`. - `pnpm typecheck`: clean. - `gh aw compile review`: 0 errors, 0 warnings. - No changeset needed: both files are under `.github/`, the check's default exclusion, and the shared `workflows/review` package is untouched. - This PR's own review does not exercise tier 2, and that is expected: for a `pull_request` event the lock runs from the head (so the lib is checked out at `review-v1.12.0`), but `restore_base_github_folders.sh` overwrites `.github/` with the base-branch snapshot and the activation job extracts the inline sub-agents from that same base markdown, so the prompt and the `## agent:` set both come from main at `review-v1.11.0`. With no `claim-clusterer` definition, dispatch leaves the run on tier 1 and records a run warning: the documented soft-degradation path. Both halves come from main once this merges. Author: jwbron Auditors: jeresig, jaredly Required Reviewers: Approved By: Checks: ⏭️ 8 checks have been skipped, ⌛ 1 check is pending, ✅ 2 checks were successful Pull Request URL: #327
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The reviewer installed on this repo (
.github/workflows/review.md) was still pinned atreview-v1.4.1, the versiongh aw addrecorded on install (#241, 2026-07-10), and had never been updated since: five releases behind the shared source (now bumped all the way to v1.7.0, released today in #278 with the Fable-correctness reviewer, the recap fixes, and the unbounded-read lens). This surfaced on PR #266, where re-review 4739236283 posted a REQUEST_CHANGES whose entire body was "Changes requested — see inline comments." with zero inline comments attached (the blocking thread lives on an earlier review). Later releases address exactly that:Why not
gh aw updateThe documented update path fails on this repo: gh-aw v0.81.6's
resolveLatestRefonly treats the pinned ref as a tag whenisSemanticVersionTag()accepts it, and that check (semverutil.IsValid) rejects changesets-style prefixed tags likereview-v1.4.1. It falls through to branch resolution and 404s (failed to get latest commit for branch review-v1.4.1). Might be worth an upstream issue; until then, updates of this workflow are manual.What this PR does
gh aw update's 3-way merge by hand: base =review-v1.4.1source, ours = installed copy, theirs =review-v1.7.0source (identical to current main), thengh aw compile review. Diff vs the v1.6.1 source is exactly the documentedKHAN/ACTIONS LOCAL OVERRIDEedits and nothing else:if:(public-repo hardening);observability:block stays commented out (theGH_AW_OTEL_SENTRY_*secrets still exist neither on this repo nor at org level; the block hard-fails the agent job when they are missing);max-ai-credits: 2500, now mirrored into the newREVIEW_MAX_AI_CREDITSenv var per the KEEP-IN-SYNC rule v1.6.0 added;source:and the lib checkoutref:both bumped toreview-v1.7.0(kept in lockstep, enforced byreview-pins.test.ts).review-pins.test.tsdiffs the installed copy againstworkflows/review/review.mdat the pinned tag (fetching the tag when the checkout is shallow) and fails when any hunk lacks aKHAN/ACTIONS LOCAL OVERRIDEmarker, so the next manual bump cannot silently drop an override or an upstream hunk.fullmode), so enabling a reduced-depth mode is a separable decision; it follows in its own PR starting atscopedrather thanflip-gated, per the review's depth argument, once a re-review or two has been observed on v1.7.0 (review: enable scoped re-reviews (follow-up to the v1.7.0 bump) #277).Verification
pnpm test: passes, including the new content guard,review-pins.test.tspin consistency (source/ref/lock literals allreview-v1.7.0), andversion-sync.test.ts. The guard was negative-tested: an unmarked stray edit to the installed copy fails it.gh aw compile review: 0 errors, 0 warnings..github/, the check's default exclusion, and the sharedworkflows/reviewpackage is untouched.